Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin/bump-readlif-for-multi-scene #327

Merged
merged 1 commit into from
Oct 3, 2021

Conversation

psobolewskiPhD
Copy link
Contributor

Description

Readlif has been update to better handle scene names that have a parent/child structure.
This change addresses the issue originally reported here: #277
Bumping the version requirement on readlif is a significant quality-of-life improvement for aicsimageio users, as well as the new multi-scene reading widget in napari-aicsimageio. For example, in the case of multi-well experiments, regions of interest may have the same names (R1, R1 ...), but the parent (well) names will differ (A1, B2, etc.). See:

Screen Shot 2021-10-03 at 5 56 54 PM

You can see the scene names are "rich", including enclosing parent information, not just "R1", "R1_Merged".

Pull request recommendations:

  • Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
  • Link to any relevant issue in the PR description. Ex: Resolves [Fix release #12], adds tiff file format support
  • Provide relevant tests for your feature or bug fix. N/A
  • Provide or update documentation for any feature added by your pull request. N/A

Thanks for contributing!

`Readlif` has been update to better handle scene names that have a parent/child structure.
This change addresses the issue originally reported here: AllenCellModeling#277
Bumping the version requirement on `readlif` is a significant quality-of-life improvement for `aicsimageio` users, as well as the new multi-scene reading widget in `napari-aicsimageio`. For example, in the case of multi-well experiments, regions of interest may have the same names (R1, R1 ...), but the parent (well) names will differ (A1, B2, etc.).
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2021

Codecov Report

Merging #327 (6789867) into main (5ea864b) will decrease coverage by 0.16%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #327      +/-   ##
==========================================
- Coverage   94.46%   94.30%   -0.17%     
==========================================
  Files          40       40              
  Lines        3018     3018              
==========================================
- Hits         2851     2846       -5     
- Misses        167      172       +5     
Impacted Files Coverage Δ
aicsimageio/tests/conftest.py 92.30% <0.00%> (-3.85%) ⬇️
aicsimageio/tests/readers/test_czi_reader.py 96.29% <0.00%> (-3.71%) ⬇️
aicsimageio/readers/ome_tiff_reader.py 98.13% <0.00%> (-0.94%) ⬇️
aicsimageio/readers/czi_reader.py 95.43% <0.00%> (-0.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ea864b...6789867. Read the comment docs.

@evamaxfield evamaxfield added the admin Various administrative tasks for the package label Oct 3, 2021
Copy link
Collaborator

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. I am surprised we don't need to change any tests?

@psobolewskiPhD
Copy link
Contributor Author

I’ve been running this readlif branch since before the widget change. I was surprised when I first worked on it that the complex scene names JustWorked™️. But they do. I guess the string processing everywhere is safe.

@evamaxfield evamaxfield merged commit 2bd5e38 into AllenCellModeling:main Oct 3, 2021
@psobolewskiPhD psobolewskiPhD deleted the patch-1 branch October 4, 2021 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin Various administrative tasks for the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants